Skip to content

Conversation

@manusa
Copy link
Member

@manusa manusa commented Jul 18, 2025

Copy link
Member

@ardaguclu ardaguclu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

const (
CustomAuthorizationHeader = "kubernetes-authorization"
OAuthAuthorizationHeader = "Authorization"
CustomAuthorizationHeader = HeaderKey("kubernetes-authorization")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why linter forces us to do this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is not at the header level, but at the usage within context.WithValue:

https://github.com/manusa/kubernetes-mcp-server/blob/b4d73d54c5fe711da26d5f53a0b2280505dcc182/pkg/mcp/mcp.go#L166

This was the first thing I thought of to solve it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

@ardaguclu
Copy link
Member

Maybe it is better to define set of linters like openshift/lws-operator@1f2f698#diff-9917ddc9f1c3304218f7269265b746d997c5c0615478177b5fceecd33ef47cb5. Not a blocker for this PR though

@manusa
Copy link
Member Author

manusa commented Jul 18, 2025

Maybe it is better to define set of linters like openshift/lws-operator@1f2f698#diff-9917ddc9f1c3304218f7269265b746d997c5c0615478177b5fceecd33ef47cb5. Not a blocker for this PR though

Whatever you think is more suitable, I added the linter because you suggested it.

Once this is in, we should also enable the linter on the CI, we an define the set in the follow-up too.

@ardaguclu
Copy link
Member

Maybe it is better to define set of linters like openshift/lws-operator@1f2f698#diff-9917ddc9f1c3304218f7269265b746d997c5c0615478177b5fceecd33ef47cb5. Not a blocker for this PR though

Whatever you think is more suitable, I added the linter because you suggested it.

Once this is in, we should also enable the linter on the CI, we an define the set in the follow-up too.

Thank you. That looks good to me.

@manusa manusa added this to the 0.1.0 milestone Jul 18, 2025 — with automated-tasks
@manusa manusa merged commit 3fbfd8d into main Jul 22, 2025
5 checks passed
@manusa manusa deleted the chore/lint branch July 22, 2025 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants